drivers: led_strip: ws2812_spi: Ensure MOSI is low during reset #99092
Conversation
zephyrbot
requested review from
jeppenodgaard,
simonguinot,
soburi and
thedjnK
arthur-mmlw
force-pushed
the
drivers-led_strip-ws2812-force-mosi-low-in-reset
branch
from
ad683ad to
d0854e0
Compare
simonguinot
left a comment
simonguinot
left a comment
There was a problem hiding this comment.
Hi @arthur-mmlw,
Thanks for this patch. But it is not the first time it is proposed. Please have a look at the links I attached in a previous comment.
And since my point of view didn't change, my comment is still the same: #75965 (comment).
|
Hello @simonguinot Sorry, I didn't see the issue had already been discussed. I'll take a look, thanks! |
|
@simonguinot I understand your point and I agree with you that adding a lot of padding bytes before and after the data frame is a waste of memory. However, I'm not using more memory in this patch. Instead, I'm relying on the fact that a I've checked this behavior in the drivers I haven’t checked every SPI driver, and for some, it's unclear from the code or comments whether dummy/NOP NULL buffers are supported. |
Seen.
It seems to me that the behavior with NULL buffers depends on SPI driver and controller. So it might work in some cases but not in others... The method you propose is different but my point is still the same: it is not up to the LED driver to handle the SPI MOSI signal at idle state. It should be controlled via the SPI subsystem. And if it can't be done, then there are plenty of alternatives (UART, I2S, GPIO). That said, I don't have a strong opinion on this subject and I will wait to see what others think. |
arthur-mmlw
force-pushed
the
drivers-led_strip-ws2812-force-mosi-low-in-reset
branch
from
d0854e0 to
11d87a5
Compare
I agree with this. I also am a bit stumped on this PR because for years, in fact even right now I have 2 STM32's (stm32f103) connected to PCs with a bunch of ws2812 LEDs which would seem impossible based on your description, yet it's been working absolutely fine for... and let me check, 3 years when I originally created the project on zephyr 2.7? |
arthur-mmlw
force-pushed
the
drivers-led_strip-ws2812-force-mosi-low-in-reset
branch
2 times, most recently
from
9695c0d to
11d87a5
Compare
zephyrbot
requested review from
FRASTM,
GeorgeCGV,
djiatsaf-st,
erwango,
etienne-lms,
gautierg-st,
juickar,
mathieuchopstm and
simonguinot
Some platforms, such as STM32, do not guarantee that the MOSI line is held low when the SPI peripheral is disabled. This can prevent WS2812-based LEDs from resetting correctly, as they require a low signal for a specific duration to trigger a reset. This commit introduces a Kconfig option to actively drive the MOSI line low during the reset period by sending zero-byte frames via SPI. This ensures the reset condition is reliably met across all hardware platforms. Signed-off-by: Arthur Gay <[email protected]>
arthur-mmlw
force-pushed
the
drivers-led_strip-ws2812-force-mosi-low-in-reset
branch
from
11d87a5 to
b9963d3
Compare
Hello @thedjnK, for more context I'm using a stm32h7rs mcu with the spi_ll_stm32 driver. The datasheet for the LED I use is : https://www.brightek.com/uploadfile/files/202312/ISA2020VGBC1CKA1-V1_y24NS.pdf. The timing are not identical to the ws2812 so it would explain why it's working for your setup but not with mine. |
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
4 participants
Some platforms, such as STM32, do not guarantee that the MOSI line is held low when the SPI peripheral is disabled. This can prevent WS2812-based LEDs from resetting correctly, as they require a low signal for a specific duration to trigger a reset.
This commit introduces a Kconfig option to actively drive the MOSI line low during the reset period by sending zero-byte frames via SPI. This ensures the reset condition is reliably met across all hardware platforms.